Skip to content

SPS#351

Open
defragator wants to merge 1 commit intorocketacademy:mainfrom
defragator:main
Open

SPS#351
defragator wants to merge 1 commit intorocketacademy:mainfrom
defragator:main

Conversation

@defragator
Copy link

Please fill out the survey before submitting the pull request. Thanks!

🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀

How many hours did you spend on this assignment?
2 hours and counting

Please fill in one error and/or error message you received while working on this assignment.
syntax errors. lots of them

What part of the assignment did you spend the most time on?
understanding the code and making sense of them on script js

Comfort Level (1-5):
3
Completeness Level (1-5):
2

What did you think of this deliverable?
Definite challenge. Enjoyable nonethe less

Is there anything in this code that you feel pleased about?
Enjoyable overall experience. Fun discovering new things

Copy link

@leechuanxin leechuanxin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey Shane, the Base logic seems quite okay on my end. A small point to note regarding user experience, it will be good to inform the players of the next game state or expected input. For instance, I did not know that the system is expecting one of 'scissors', 'stone', or 'paper' as the input the moment a round finishes.

Also, you may not want to increment count in line 11. It may be better to increase it when the game is actually functioning properly (eg. no invalid input). Otherwise, count is going to keep increasing despite entering invalid input like "bananas".

Thus, I recommend increasing count in the function between line 19 to 37 instead.

n = input;
return 'Hello ' + input;
}
++count;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally we prefer count += 1 instead of count++ or ++count. The ++ and -- syntaxes are very prone to human error, and you can learn more about this here: https://eslint.org/docs/rules/no-plusplus

@@ -1,4 +1,37 @@
var n = null;
Copy link

@leechuanxin leechuanxin Nov 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like n is ultimately not used anywhere, so it can be removed. While n is assigned to input in line 8, it doesn't look like you are returning n in the string in line 9.

If you are intending to use n to meet any user name requirement, it will be good to carry the user name string over to the output on lines 16, 27, 33, and 36.

Additionally, it will be better to name this variable clearly, for other readers / developers collaborating with you. username will be better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants